node:http2: encode each header block in one call, all fields or none - #41520

How a block failed after its first fields were in the table

1. A later field fails validation and the call throws: an invalid value or name, undefined, null, a Symbol, a toString that throws. A user try/catch is not needed for this: pushStream() and a request that was queued before 'connect' catch the native throw themselves.
2. The encoder refuses a later field, because the name plus the value is longer than 65536 bytes. The call returns. The session error (code 9) arrives from the event loop. Blocks that other streams send in the same tick go out first, with shifted indices.
3. request() refuses the block after the field walk: an option check, the session memory limit, maxSendHeaderBlockLength. On main these checks ran after the encode.
4. A header value's toString() makes another header call on the same session. That block went out ahead of the fields that the first call had already encoded.

Repro for the first way (bun 1.4.3, bun client and bun server):

import http2 from "node:http2";
const srv = http2.createServer();
srv.on("stream", (st, h) => {
  if (h[":path"] === "/poison") {
    try { st.additionalHeaders({ ":status": 103, "x-a": "AAAA", "x-v": "a\r\nb" }); } catch (e) { console.log("S threw", e.code); }
    st.respond({ ":status": 200, "x-after-info": "1" }); st.end("p");
  } else { st.respond({ ":status": 200, "x-clean": "clean-value", "content-type": "text/plain" }); st.end("c"); }
});
srv.listen(0, async () => {
  const s = http2.connect("http://127.0.0.1:" + srv.address().port);
  s.on("error", e => console.log("C sess", e.code));
  const get = p => new Promise(r => { const q = s.request({ ":path": p }); q.on("response", h => { delete h.date; console.log("C", p, JSON.stringify(h)); }); q.resume(); q.on("error", e => console.log("C", p, "err", e.code)); q.on("close", r); });
  await get("/clean1"); await get("/poison"); await get("/clean2"); s.close(); srv.close();
});

Before:

C /clean1 {":status":200,"x-clean":"clean-value","content-type":"text/plain"}
S threw ERR_HTTP2_INVALID_HEADER_VALUE
C /poison {":status":200,"x-after-info":"1","x-clean":"clean-value"}
C sess ERR_HTTP2_ERROR
C /clean2 err ERR_HTTP2_ERROR

After:

C /clean1 {":status":200,"x-clean":"clean-value","content-type":"text/plain"}
S threw ERR_HTTP2_INVALID_HEADER_VALUE
C /poison {":status":200,"x-after-info":"1"}
C /clean2 {":status":200,"x-clean":"clean-value","content-type":"text/plain"}

This bug has no user report. An automated fuzz run found it. The table invariant itself has precedent: #31323 fixed it on the decode side, and the HTTP/2 client of fetch stops using a connection after a failed encode (encoder_poisoned).

What a refused block does now

• A field over 65536 bytes in respond(), additionalHeaders(), pushStream(), request(), compat writeHead() or writeEarlyHints(): the session still ends with ERR_HTTP2_SESSION_ERROR: Session closed with error code 9 one tick later, as before. Blocks that other streams send in that tick now decode right. Node's encoder does not have this limit, so the rows that expect code 9 pin a known difference.
• A field over 65536 bytes in sendTrailers(): frameError, the stream ends with FRAME_SIZE_ERROR, then a graceful GOAWAY, as before.
• maxSendHeaderBlockLength: the check uses nghttp2's bound (12, plus 12 per field, plus the name and value bytes, plus 5) and runs before the encode.
  • With a limit of 300 and respond({ ":status": 200, "x-a": <N bytes> }): N up to 246 is sent by main, by this PR and by node v26.3.0. N from 247 to 465: main sends the response, this PR and node reset the stream. N from 466: main sends nothing and the request hangs, this PR and node reset the stream.
  • A refused server block dispatches frameError and resets the stream with FRAME_SIZE_ERROR. Without the reset, the 219 lengths from 247 to 465 go from a 200 to no answer.
  • additionalHeaders() marks its block with a new argument. A refused block of that call leaves the stream open, so a respond() still goes out. node:http2: refuse a header block over the default send limit, like node #43474 has the same argument.
  • The client side reacts as on main: frameError, then the stream fails with REFUSED_STREAM.
• A throw from ClientHttp2Session.request() is no longer reported a second time as a session 'error'. Nothing reached the wire, and node reports the throw only. When the failed call also closed the session, the session is now destroyed without that error (GOAWAY code 0, it was code 2).

Measurements

Release builds of main (bbdc5a5) and of this PR's source on that base (b9d682a), linux x64, same toolchain. The container has no perf, valgrind, strace, ltrace or bloaty. The counts come from gdb: breakpoints on the allocator entry points and single steps, counted only inside the host function, in steady state.

• Allocator calls per call (mi_malloc calls, bytes requested):
  • respond(), 7 fields: 19 (17,136 B) on main, 18 (752 B) on the PR
  • client request(): 27 (17,328 B), then 26 (944 B)
  • sendTrailers(), 2 fields: 7 (16,688 B), then 6 (304 B)
  • pushStream(), 5 fields: 15 and 1 mi_realloc, then 14 and 0
  • additionalHeaders(): 9, then 8
  • The first header call in a VM allocates the scratch. Later calls reuse it.
• Instructions per call, callees included (a second run of each differs by less than 0.4%):
  • respond(), 7 fields: 27,947, then 28,333 (+1.4%)
  • client request(): 33,361, then 33,748 (+1.2%)
  • sendTrailers(): 14,183, then 14,105 (-0.5%)
  • pushStream(): 21,602, then 21,522 (-0.4%)
• Copies: one copy of each name and value on both builds. Main also zero-fills name + value + 32 bytes per field before it encodes.
• Code size of the three host functions and their helpers: 26,028 B, then 27,537 B (+1,509 B). .text: 65,382,485 B, then 65,385,557 B (+3,072 B). The stripped binary: 88,864,328 B, then 88,868,424 B (+4,096 B, one page).
• Stack frames of the three host functions: 4,664, 4,552 and 4,640 B, then 4,800, 4,600 and 4,672 B.
• Host functions: 29 on both. lshpack_wrapper_* symbols: 2, then 3.
• size_of::<H2HeaderScratch>() is 72, so RareData grows by 72 B. H2FrameParser has no new field (1,496 B). The slot keeps a scratch only when each of its two buffers is at most 64 KiB.
• Wall clock, measured on an earlier build of this PR: user CPU for 200,000 request and response pairs, 10 interleaved runs, median 10.995 s on main and 11.013 s on the PR (+0.16%). The noise floor is 1.65%, so this shows no difference.

Tests

test/js/node/http2/node-http2-header-block.test.ts is new and has 62 rows. 59 fail on bun 1.4.3-canary.1. The rows have a file of their own because they share no helper with node-http2.test.js, which has 7,700 lines.

• "a header call that throws leaves the HPACK encoder in sync with the peer" (42 rows). Seven kinds of bad field across additionalHeaders, respond, compat res.end, server and client sendTrailers, pushStream and client request. Each row makes a clean request, the failing call, and a clean request on the same session. One row queues the failing request before 'connect'. One row closes the session from inside the failing request(). That row fails when the new line in the catch of request() is deleted. The invalid-name rows of respond() and of client request() pass on main. Two kinds (undefined, null) are values that node accepts. node:http2: normalize outbound header objects like node #41614 changes those.
• "a header call made from a header value's toString() during another header call" (7 rows). Four server calls whose value hook answers another stream, and three client shapes with sendTrailers().
• "a block refused after its fields are read does not reach the HPACK table" (4 rows): the client's maxSendHeaderBlockLength (the follow-up requests are made in the same tick), the client's maxSessionMemory, an option that is out of range, and a respond() whose options getter throws.
• "a header block the encoder refuses does not reach the HPACK table" (8 rows). Six server calls send a fresh field and then a field of 65537 bytes. Three held streams get their answers in the same tick and must decode right. One row does the same with a client request(). One row reads raw frames: a field of exactly 65536 bytes is sent, and the next block is indexed against it. That row passes on main. The sessions raise maxSendHeaderBlockLength, as grpc-js does, so the rows keep their meaning when a default send limit lands.
• "maxSendHeaderBlockLength on a server response" (1 row). With a limit of 300, a 246-byte value is sent, a 247-byte value resets the stream, and a refused additionalHeaders() block leaves the stream open. Node v26.3.0 gives the same result for the same script.

Also run on a debug build of the PR: all of test/js/node/http2/ (658 pass, 6 skip with --timeout 120000) and node's 256 test-http2-*.js files.

Not in this PR

• A field over 65536 bytes still ends the session with code 9, and that cuts the streams in flight on the connection. Node refuses such a block at its default send limit of 64 KiB: it resets that one stream, closes the session gracefully, and lets the other streams finish. node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength #43440, node:http2: refuse a header block over the default send limit, like node #43474, node:http2: fail only the push when a pushStream() header block is over the send limit #43619 and node:http2: refuse a trailer block over the send limit #43632 work on that limit. They cover default sessions. A session that raises the limit (grpc-js does) needs encode_block to take every field, with a literal without indexing for a field that lshpack refuses.
• Node closes the stream and the session one setImmediate after a frame error. This PR resets a refused response at once, never resets a refused additionalHeaders() block, and does not close the session. node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength #43440 has node's rule.
• After that reset, a compat res.end() in the same tick still writes one empty DATA frame for the stream. Main writes that frame with no HEADERS before it.
• maxSendHeaderBlockLength is still not applied to sendTrailers() and pushStream().
• A refused sendTrailers() block still ends the other streams of the session early, with END_STREAM. The table stays in sync. Main has the same bug.
• Frames that are written while one session takes the cork buffer from another are still dropped. The flush of the other session can run its transport's _write, and what that JS writes is lost. A header block that is lost that way leaves its fields in the table. An earlier version of this PR kept those frames with a loop in cork(). The self-review found that the loop has no bound and that the kept frames can go out of order, so that change is out again. node:http2: remove unsafe from H2FrameParser #40240 has a hand-over that runs no JS, and tls: consolidate the open TLS fixes (both engines, node:tls, node:https, WebSocket, SQL) #44618 has the write() routing for a socket on top of a JS Duplex.
• A request() or pushStream() that a header value's toString() makes from inside another request() still reaches the wire with the higher stream id first. node:http2: coerce outbound header values before taking a stream id #37568 works on that.
• The encoder still ignores the peer's SETTINGS_HEADER_TABLE_SIZE (Bun's HTTP/2 server potentially ignores Nginx's HPACK for header compression setting SETTINGS_HEADER_TABLE_SIZE=0 #19152, node:http2: apply SETTINGS_HEADER_TABLE_SIZE to the HPACK encoder and decoder #43345). After this PR that update has one place to go: encode_header_list.
• The HTTP/2 client of fetch (src/http/h2_client/) keeps its per-field encode. After a failed encode it opens no new stream on that connection (encoder_poisoned), so no later block uses the shifted table.
• If lshpack cannot allocate in the middle of a block, the process aborts, because the table cannot be rolled back.
• The engine in src/runtime/api/bun/h2/ uses the block encode too: send_header_block() and send_push_promise() take the staged block. Nothing calls that outbound half yet. Its unit tests are compile-checked only (cargo check -p bun_runtime --lib --tests).

Related PRs and landing order

• node:http2: don't let a header block that fails validation reach the HPACK encoder #33470 is the earlier attempt at the validation case, closed in favour of this PR.
• node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength #43440 and the PRs stacked on it (node:http2: refuse a header block over the default send limit, like node #43474, node:http2: fail only the push when a pushStream() header block is over the send limit #43619, node:http2: refuse a trailer block over the send limit #43632) change the same three functions. They stage fields in their own HeaderList and then still encode field by field. After this PR they can use HeaderBlock::deflate_bound() and drop that type. node:http2: reset the stream when a response header block is over maxSendHeaderBlockLength #43523 also resets a refused response. This PR has the reset of node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength #43440 and the informational argument of node:http2: refuse a header block over the default send limit, like node #43474, because the moved check needs them. A maintainer has to set the order: the PR that lands second needs a rebase.
• node:http2: normalize outbound header objects like node #41614 is stacked on this branch.
• node:http2: apply SETTINGS_HEADER_TABLE_SIZE to the HPACK encoder and decoder #43345 and node:http2: coerce outbound header values before taking a stream id #37568: see "Not in this PR".

Self-review

32 concerns raised, 28 addressed in the code, the tests or this text. Not taken:

1. Remove the server reset and keep main's reaction. Without a reset, the lengths 247 to 465 above go from a 200 to no answer.
2. Add node's close of the stream and the session after a frame error. That is the subject of node:http2: reset the stream when respond() exceeds maxSendHeaderBlockLength #43440.
3. Apply the peer's SETTINGS_HEADER_TABLE_SIZE here. That is the subject of node:http2: apply SETTINGS_HEADER_TABLE_SIZE to the HPACK encoder and decoder #43345.
4. Take the stream id after the header walk. That is the subject of node:http2: coerce outbound header values before taking a stream id #37568.

Rows for respondWithFile() and for a raw header array that throws were suggested and are not added.

History

The first version of this PR (September, 90f334a) fixed the validation case only. Its text said that an encoder failure needs no pre-check, because it ends the session. That statement was not tested and it is wrong: see the second way above. A later version (57fa470) also changed cork() and write(). Those changes are out again, see "Not in this PR".